I tested GLPK.jl and it passed, but I didn't run the MOI tests. I don't really see why we shouldn't throw here. We've already figured out we're going to throw an error, so why not now? There should be other tests that catch incorrect usage in other parts of the API.

Can't the if !supports(...) be removed as well, i.e., so that it completely reverts #1614 ?

We've already figured out we're going to throw an error, so why not now?

If you completely revert it, "We've already figured out" won't apply anymore

We still need to be able to skip names and start values, so we need the !MOI.supports.

Bump @blegat

Indeed, the PR moved this if but it was already there.

I don't really see why we shouldn't throw here. We've already figured out we're going to throw an error, so why not now?

That's one way to read this but when I read this code, I don't understand why we throw since set is going to throw. Every line in a code should be there for a reason and here it's confusing because it doesn't have to be there.

There should be other tests that catch incorrect usage in other parts of the API.

The top PR comment of #1613 says

throw the right error so down-stream users don't have to duplicate work.

So the motivation was that set might not work. If we assume set is working then there is no reason to throw here.

So is that a yay/nay on merging this then?

Ah. I found the problem:

using SCS
const MOI = SCS.MOI
model = MOI.instantiate(SCS.Optimizer, with_bridge_type = Float64)
MOI.copy_to(model, MOI.Test.BadModelAttributeModel())  # No error. Should throw unsupported attribute

Why? Because the model has a cache:

julia> model
MOIB.LazyBridgeOptimizer{MOIU.CachingOptimizer{SCS.Optimizer, MOIU.UniversalFallback{MOIU.Model{Float64}}}}
with 2 variable bridges
with 0 constraint bridges
with 0 objective bridges
with inner model MOIU.CachingOptimizer{SCS.Optimizer, MOIU.UniversalFallback{MOIU.Model{Float64}}}
  in state EMPTY_OPTIMIZER
  in mode AUTOMATIC
  with model cache MOIU.UniversalFallback{MOIU.Model{Float64}}
    with 1 model attribute
    fallback for MOIU.Model{Float64}
  with optimizer SCS.Optimizer

We'll throw the error on optimize!, but not on copy_to.

I think we should revert this PR; it broke tests for SCS.

Yeah, this caused pretty widespread breakage in the SolverTests: https://github.com/jump-dev/SolverTests/runs/4316646996?check_suite_focus=true